Skip to content

Dataflow: Restrict partial flow to either forward or reverse flow.#14599

Merged
MathiasVP merged 3 commits intogithub:mainfrom
aschackmull:dataflow/partialflow-separate
Oct 26, 2023
Merged

Dataflow: Restrict partial flow to either forward or reverse flow.#14599
MathiasVP merged 3 commits intogithub:mainfrom
aschackmull:dataflow/partialflow-separate

Conversation

@aschackmull
Copy link
Copy Markdown
Contributor

@aschackmull aschackmull commented Oct 26, 2023

When using partial flow we would calculate both partial forward flow and partial reverse flow even though only one direction was requested. This moves the choice into the parameterised module application so we know up front which one to calculate. So instead of always using module Partial = FlowExploration<limit/0> and choosing between Partial::partialFlow and Partial::partialFlowRev, you now choose between module Partial = FlowExplorationFwd<limit/0> and module Partial = FlowExplorationRev<limit/0>, and then always use Partial::partialFlow.

MathiasVP
MathiasVP previously approved these changes Oct 26, 2023
Copy link
Copy Markdown
Contributor

@MathiasVP MathiasVP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I guess we should also update the reference to FlowExploration in https://codeql.github.com/docs/writing-codeql-queries/debugging-data-flow-queries-using-partial-flow/ (or make FlowExploration an alias for FlowExplorationFwd)?

@aschackmull
Copy link
Copy Markdown
Contributor Author

I guess we should also update the reference to FlowExploration

The doc needs an update regardless as the info about reverse flow needs a fix.

Copy link
Copy Markdown
Contributor

@MathiasVP MathiasVP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code changes LGTM! Should we have the Docs team 👀 the documentation changes?

@aschackmull
Copy link
Copy Markdown
Contributor Author

Should we have the Docs team 👀 the documentation changes?

It's very minor, so I think your review is plenty.

@MathiasVP MathiasVP merged commit b1d4ca5 into github:main Oct 26, 2023
@aschackmull aschackmull deleted the dataflow/partialflow-separate branch October 26, 2023 10:22
@alexet
Copy link
Copy Markdown
Contributor

alexet commented Oct 26, 2023

Do you need to update the test queries in java/ql/test/library-tests/dataflow/partial?

@aschackmull
Copy link
Copy Markdown
Contributor Author

Do you need to update the test queries in java/ql/test/library-tests/dataflow/partial?

Gah! Of course CI didn't run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants